-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
installation: untar libzmq in venv folder instead of /usr/local #2305
installation: untar libzmq in venv folder instead of /usr/local #2305
Conversation
notapirate
commented
Mar 21, 2024
- on a linux-host docker build, the user can't write to /usr/local and installing pyzmq fails
- untar libzmq to venv instead
- normal installation also changed because it's sensible to have all pyzmq dependencies in the venv
- jukebox.Dockerfile: removing ZMQ_PREFIX=${PREFIX} since PREFIX variable doesn't exist in docker environment
- jukebox.Dockerfile: use ENV to set ZMQ_DRAFT_API
227795b
to
8e74ebe
Compare
- on a linux-host docker build, the user can't write to /usr/local and installing pyzmq fails - normal installation also changed because it's sensible to have all pyzmq dependencies in the venv - this also makes the separate 'sudo rsync' command obsolete - jukebox.Dockerfile: removing ZMQ_PREFIX=${PREFIX} since PREFIX variable doesn't exist in docker environment - jukebox.Dockerfile: use ENV to set ZMQ_DRAFT_API
8e74ebe
to
54d3baa
Compare
Hi, thanks for your contribution. My thoughts about this:
|
I'm not a python expert, so if you say that's something one doesn't do that's fine with me. My reasoning was that libzmq, though not a python package, is a python package dependency, but I haven't found a clear rule/suggestion on the web whether non-python dependencies should be put in the venv or not, so I'll just use your suggestion to untar as root to /usr/local in docker and leave the normal installation unchanged.
OK so how does this work, I make the changes and create a merge request with MR #2167? |
PR #2307 works and fixes this issue, so I'm closing this PR, thanks @AlvinSchiller for your time to review this PR. |